-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate and replace some Thrust iterator traits #3928
Conversation
🟨 CI finished in 1h 19m: Pass: 3%/93 | Total: 16h 31m | Avg: 10m 39s | Max: 1h 17m | Hits: 97%/308
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
# | Runner |
---|---|
66 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
4 | linux-arm64-cpu16 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
396f5a4
to
cd467d5
Compare
template <typename It> | ||
using it_difference_t = typename | ||
// FIXME(bgruber): switching to ::cuda::std::iterator_traits<T> breaks some tests, e.g., | ||
// thrust.test.tabulate_output_iterator | ||
#if _CCCL_COMPILER(NVRTC) | ||
# include <cuda/std/iterator> | ||
#else // _CCCL_COMPILER(NVRTC) | ||
# include <iterator> | ||
::cuda | ||
#endif // _CCCL_COMPILER(NVRTC) | ||
::std::iterator_traits<It>::difference_type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miscco I may need your help here, please. Not necessarily for this PR, but we should figure out what's going on before CCCL 3.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is ::cuda::std::iterator_traits
trying to figure out some common_reference
of zip_iterator
when determining cuda::std::iterator_traits<zip_iterator<...>>
, which I think shouldn't be needed. it works with std::iterator_traits
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you try std::iterator_traits
in C++20 mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The answer is most likely that zip_iterator
is cursed and we should add a specialization of iterator_traits
for it
looking at you tuple_of_iterator_references
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW I hope the issues are compile-time issues not silent runtime degradation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you try
std::iterator_traits
in C++20 mode?
Yes, it works. Tested locally and this is also what the CI should run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a reproducer: https://godbolt.org/z/7jb4qG3bb
🟨 CI finished in 1h 48m: Pass: 90%/93 | Total: 2d 19h | Avg: 43m 39s | Max: 1h 31m | Hits: 47%/120894
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
# | Runner |
---|---|
66 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
4 | linux-arm64-cpu16 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
cd467d5
to
599c5fb
Compare
🟩 CI finished in 1h 47m: Pass: 100%/93 | Total: 2d 05h | Avg: 34m 18s | Max: 1h 19m | Hits: 69%/133929
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
# | Runner |
---|---|
66 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
4 | linux-arm64-cpu16 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
Deprecate and remove use of thrust:: * iterator_traits * iterator_value * iterator_value_t * iterator_pointer * iterator_pointer_t * iterator_reference * iterator_reference_t * iterator_difference * iterator_difference_t Add as replacement thrust::detail:: * it_value_t * it_reference_t * it_difference_t * it_pointer_t
341cafd
to
a71e67d
Compare
🟨 CI finished in 56m 19s: Pass: 51%/93 | Total: 14h 55m | Avg: 9m 37s | Max: 55m 35s | Hits: 92%/53793
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
# | Runner |
---|---|
66 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
4 | linux-arm64-cpu16 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
a71e67d
to
c45663a
Compare
🟩 CI finished in 1h 09m: Pass: 100%/93 | Total: 15h 56m | Avg: 10m 17s | Max: 50m 47s | Hits: 94%/133929
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
# | Runner |
---|---|
66 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
4 | linux-arm64-cpu16 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
This PR is similar to #3924, but for Thrust.
We add:
and replace all uses of
thrust::iterator_xxx
traits which have a replacement in libcu++. Then, those thrust traits are deprecated.The naming choice rational is here: